Skip to content

Use Spoteless for header checking and code-formatting #721

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 20, 2025

Conversation

ricardozanini
Copy link
Member

Many thanks for submitting your Pull Request ❤️!

What this PR does / why we need it:
In this PR:

  1. We replaced the old Spotify plugin and replaced header checks with Spotless.
  2. Spotless now adds headers automatically to Java files
  3. Added a Makefile to facilitate the local dev flow (run make help)
  4. Added a pre-commit hook to format/add headers to new files in case you missed
  5. Refactored the CONTRIB guidelines

Special notes for reviewers:
@fjtirado if you agree, feel free to merge.

Additional information (if needed):
Since the project is growing, I believe we should start to improve the governance.

PS: I intend to change this formatter. The Google one is too narrow IMO.

@ricardozanini
Copy link
Member Author

I'll fix the error tomorrow. Actually, there's a bug on our CI. The verify examples is not running the samples. 😆

@fjtirado
Copy link
Collaborator

fjtirado commented Aug 20, 2025

@ricardozanini I did a a quick check using Eclipse (I was not using command line for anything till now) and it was not good (I have to start using command line)

After executing make hooks (which is not automatized by Eclipse, so command line), I added a file without header in Eclipse, add to stage the new file and the header was not added. I still has to add it manually or execute make checks (I did not find a way to invoke make checks from Eclipse, I have to use the command line. Funny thing is that I realized I have not had the need to install maven till now, it was enough with the Eclipse embedded one)
So, basically this whole change complicated usability for Eclipse IDE users, forcing them to run make hook (just once) and make check ( for every small change) from the command line. And for users that do not have command line maven installed because they were using eclipse embedded one, to install it.
Finally, and this is showstopper to me, running mvn clean install at repo root directory works, but mvn install at not root directory always fails with this error
Failed to execute goal com.diffplug.spotless:spotless-maven-plugin:2.43.0:check (default) on project serverlessworkflow-api: Execution default of goal com.diffplug.spotless:spotless-maven-plugin:2.43.0:check failed: Unable to locate file with path: /home/ftirados/git/sdk-java/api/config/license-header.txt: Could not find resource '/home/ftirados/git/sdk-java/api/config/license-header.txt'. -> [Help 1]
Using ${maven.multiModuleProjectDirectory} as env variable is not a good option if you have several multi module projects as it usually happens. Also env variables usage within Eclipse is tricky (either you set bashrc to have env global or you have to customize every maven run)
I agree that adding headers files for new files is boring and we should automatize it, but I feel we should do that in a more user friendly way , this approach has to many glitches in my opinion.
Why not continue delegating the format to the proper maven phase as it was before and see if maven itself can add the header file if missed?
Mixing maven and git hooks is not a good approach in my opinion.

@ricardozanini
Copy link
Member Author

Hey @fjtirado thanks for testing it!

  1. I removed the githook and the makefile
  2. I can confirm that mvn clean install works everywhere now (license headers are inline in the pom.xml). I'd like to avoid inlining it, but it's simpler since Maven won't collaborate to take the parent's pom dir instead of the current one.
  3. mvn clean install will also mutate your files
  4. CI will check for any formatting/header issues before building, so we don't merge unformatted files.

@ricardozanini ricardozanini merged commit dd38a9f into serverlessworkflow:main Aug 20, 2025
3 checks passed
@ricardozanini ricardozanini deleted the fix-formatter branch August 20, 2025 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants